Skip to content

Bump pin: PoW range checks + witness reserved-value guard (blocked on schema#72) - #7

Draft
melvincarvalho wants to merge 2 commits into
gh-pagesfrom
bump-pin-pow-fix
Draft

Bump pin: PoW range checks + witness reserved-value guard (blocked on schema#72)#7
melvincarvalho wants to merge 2 commits into
gh-pagesfrom
bump-pin-pow-fix

Conversation

@melvincarvalho

Copy link
Copy Markdown
Contributor

Draft — do not merge until bitcoin-desktop/schema#72 lands. The PIN comment needs the merge SHA.

Vendors the fixes for #5 and #6 into the generated snapshot.

File Change
codec/codec.js checkProofOfWork applies Core's target range checks (powLimit, overflow, negative, zero) before comparing the hash; adds expandCompactChecked and Codec.setChainParams
codec/headers.js HeaderEngine.fromSchemas binds the network ceiling onto the codec, so SpvEngine gets it with no signature change
codec/blocks.js guards a coinbase witness reserved value that is not exactly 32 bytes — previously an uncaught RangeError
tools/sync-kernel.mjs holds the createKernel() literal, so the network parameter and setChainParams call live here too; PIN updated
packages/kernel/index.js regenerated output, matches the literal above

Why this wasn't produced by re-running the sync tool

sync-kernel.mjs's wasm-loader rewrite is not idempotent:

.replace(/loadSecpWasm\(url = [^)]*\)/, "loadSecpWasm(url = new URL('./secp256k1.wasm', import.meta.url).href)")

[^)]* stops at the first ), so run against already-transformed output it matches loadSecpWasm(url = new URL('./secp256k1.wasm', import.meta.url) and produces ...import.meta.url).href).href). Since the canonical wasm source directory isn't present in a plain checkout, re-running requires feeding it the generated copy, which mangles the loader.

So wasm/ is untouched and the three codec files are copied verbatim — exactly what a clean sync emits. The regex is worth fixing separately; it makes the tool unsafe to re-run against its own output.

No schema/*.jsonld changed in #72, so no schema modules regenerate.

Verification

Full suite green. createKernel() now carries the mainnet ceiling, and the forged SPV proof from #5 is rejected:

powLimit set  : true
forged spv ok : false

Closes #5 and #6 once merged.

Vendors bitcoin-desktop/schema PR #72 into the generated snapshot.

  - codec/codec.js    checkProofOfWork now applies Core's target range checks
                      (powLimit bound, overflow, negative, zero) before
                      comparing the hash; adds expandCompactChecked and
                      Codec.setChainParams
  - codec/headers.js  HeaderEngine.fromSchemas binds the network ceiling onto
                      the codec, so SpvEngine gets it without a signature change
  - codec/blocks.js   guards a coinbase witness reserved value that is not
                      exactly 32 bytes, which previously threw RangeError

tools/sync-kernel.mjs carries the createKernel() literal, so the network
parameter and setChainParams call are updated there too; packages/kernel/index.js
is the regenerated output and matches.

Not produced by re-running sync-kernel.mjs: its wasm-loader rewrite is not
idempotent (the `loadSecpWasm(url = [^)]*)` pattern re-matches its own output and
mangles it), so wasm/ was left untouched and the three codec files were copied
verbatim, which is what a clean sync would emit. Worth fixing that regex
separately. No schema/*.jsonld changed in #72, so no schema modules regenerate.

DO NOT MERGE until #72 lands; the PIN comment needs the merge SHA.

Refs #5, #6
The replacement text contains a ')' inside new URL(...), and the pattern
[^)]* stops at the first one — so the regex matched its own output. Running
the tool against an already-transformed loader produced

  loadSecpWasm(url = new URL('./secp256k1.wasm', import.meta.url).href).href)

a syntax error that takes the whole package with it, written with no warning
and the usual success line.

This matters because WASM_DIR defaults to a sibling checkout absent from a
plain clone, so the obvious workaround when regenerating is to point it at the
existing packages/kernel/wasm — exactly the case that corrupts.

Skips the rewrite when the loader is already in the target form, and now
throws rather than silently emitting an unrewritten loader that would resolve
the wasm against the page URL instead of the module.

Closes #8
@melvincarvalho

Copy link
Copy Markdown
Contributor Author

Pushed the fix for #8 onto this branch rather than a separate PR, since both changes touch tools/sync-kernel.mjs and would otherwise conflict.

The wasm-loader rewrite is now idempotent — it skips when the loader is already in the target form, and throws if neither the source nor the target form is present instead of silently emitting an unrewritten loader.

Verified as a fixed point:

pass1: export function loadSecpWasm(url = new URL('./secp256k1.wasm', import.meta.url).href) {
stable across further passes: true
throws when no default present: no default

With this in, the tool is safe to re-run against its own output — so once schema#72 lands, the pin bump can be done the proper way (SCHEMA_DIR=... node tools/sync-kernel.mjs) and the result should match what this branch already contains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

checkProofOfWork accepts any target: no powLimit bound, no overflow check

1 participant